✨ Add in-place update hooks to API #12343
Conversation
k8s-ci-robot
added
area/runtime-sdk
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/L
alexander-demicev
force-pushed
the
inplacehooks
branch
from
a180b63 to
9310ebd
Compare
There was a problem hiding this comment.
Thanks for this PR
TBH, It is a little bit complex to think about all the implications without a deeper understanding of when/how those hooks are being called. e.g.
- is []string the best way to represent changes (e.g. how this will work when there changes to items in an array)
- do we need more than []string? e.g. reference to KCP/MD, corresponding templates, may be list or reference to controlled machines impacted by the change
- do we need something to link a set of proposed/accepted changes to in-place updates of the corresponding machines? (e.g. what will happen if the spec changes again in the meantime)
Considering I don't have a good answer to those questions yet, my gut feeling is that we should first look into where/how hooks are going to be called and then use learning to finalize & implement hooks, also because without the first part, hooks are not usable.
But no strong opinions, I'm also ok in merging a first release and then iterate, but this will probably require us to make exceptions e.g. if breaking changes will be required (probably not a blocker in this case, but I just want to bring this up).
| // until it reports Done or Failed status. | ||
| func UpdateMachine(*UpdateMachineRequest, *UpdateMachineResponse) {} | ||
|
|
||
| func init() { |
There was a problem hiding this comment.
We probably need to start working to a book page with this documentation, but considering there are still details to be figured out, I'm also ok to defer
alexander-demicev
force-pushed
the
inplacehooks
branch
from
9310ebd to
d9e88e3
Compare
|
@fabriziopandini Thank you for the great feedback, all your points are valid. We need to start somewhere with the implementation, and my intention is to split it into smaller PRs to make the review process much easier. The in-place update feature will be hidden behind a feature gate and won’t be announced as alpha until we all agree on it. I’ll start implementing a reference updater as soon as possible, and at the same time, we’re going to begin building an experimental updater in Rancher to battle-test the idea. Hopefully, this will help clarify some of the open questions as we go. Regarding the book, I completely agree that we need to create a dedicated page. I can open an issue to track it, but I’d prefer to wait a bit until we have some functional code to document. |
Signed-off-by: Alexandr Demicev <alexandr.demicev@suse.com>
alexander-demicev
force-pushed
the
inplacehooks
branch
from
d9e88e3 to
526da98
Compare
k8s-ci-robot
added
size/XL
What this PR does / why we need it:
This PR introduces the runtime hooks infrastructure for in-place updates as defined in the In-place updates proposal
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
/area runtime-sdk